BIP54: Consensus Cleanup test vectors #2015
Conversation
This introduces a set of test vectors for each of the 4 mitigations in the BIP. The sigops and transaction size vectors were generated using the unit tests included with the Bitcoin Core implementation of BIP54, available at https://github.com/darosior/bitcoin/tree/2509_inquisition_consensus_cleanup. The timestamps and coinbases required mainnet blocks for maximum compatibility, and were generated by two dedicated unit tests not included with the Bitcoin Core implementation above but available at https://github.com/darosior/bitcoin/tree/bip54_miner.
There was a problem hiding this comment.
Concept ACK, successfully built the test branch at https://github.com/darosior/bitcoin/blob/2509_inquisition_consensus_cleanup and ran the unit and functional tests.
$ ./build/bin/test_bitcoin --run_test=bip54_tests
Running 4 test cases...
*** No errors detected
$ ./build/test/functional/feature_bip54.py
2025-10-21T16:05:13.483000Z TestFramework (INFO): PRNG seed is: 80635940825195715
2025-10-21T16:05:13.483000Z TestFramework (INFO): Initializing test directory /var/folders/bz/mn3hr6td37bczwp7j89frs4w0000gn/T/bitcoin_func_test_dznr0x1z
2025-10-21T16:05:14.300000Z TestFramework (INFO): BIP54 tests before activation
2025-10-21T16:05:14.558000Z TestFramework (INFO): Activating BIP54
2025-10-21T16:05:14.745000Z TestFramework (INFO): BIP54 tests after activation
2025-10-21T16:05:15.296000Z TestFramework (INFO): Stopping nodes
2025-10-21T16:05:15.460000Z TestFramework (INFO): Cleaning up /var/folders/bz/mn3hr6td37bczwp7j89frs4w0000gn/T/bitcoin_func_test_dznr0x1z on exit
2025-10-21T16:05:15.460000Z TestFramework (INFO): Tests successful
Are the functional tests worth mentioning in the test_vectors README? Perhaps with some of the content in the commit message:
The previously introduced unit tests extensively test the specific implementation of each
mitigation. This functional test complements them by sanity checking all mitigations in a "black
box" manner. For the added timestamp constraints, it mimicks how they would get exploited (by
implementing pseudo timewarp and Murch-Zawy attacks) and demonstrates those exploits are not
possible anymore after BIP54 activates.
darosior
force-pushed
the
2509_consensus_cleanup_test_vectors
branch
from
2fead0c to
0777a81
Compare
No, i don't think so. The functional test sanity checks the implementation with a blackbox approach, but does not implement test vectors. Hence they don't need to be mentioned in the test vectors README. |
There was a problem hiding this comment.
Started to review the code and test coverage on the branch in bitcoin-inquisition.
For the current bip-0054.md, maybe it could be valuable to have an editorial refinement of the “Specification” section where the new rules are each categorized in its section (“difficulty adjustement fix”, “long block validation time fix”, "merkle tree malleability fix” and “duplicate coinbase tx”) as it’s done for the test-vectors/README.md”. Believing it’s fruitful to review that for each line of specification, and corresponding line of code, there is adequate test coverage.
| features a chain of mainnet headers starting from the genesis block, and whether this header chain | ||
| is valid by BIP54 rules. Each test case also contains a comment describing why this particular chain | ||
| is (in)valid according to BIP54. All test cases are valid according to current Bitcoin consensus | ||
| rules. It is intended to be used to test a BIP54 implementation by feeding the header chain to a |
There was a problem hiding this comment.
Started to look on the timestamps.json file, I’m understanding that the headers for the 9 chain of headers have been drawn the historical mainnet chain, and then permutation of a subset of those chain of headers have been made accordingly to make some of them (in)valid according to BIP54, assuming that BIP54 would have been enforced as a rule since the genesis block.
Given the difficulty adjustment is proposing rules for the validation state where block is equal to height N % 2016 equal 0 and the state where block is equal to height N % 2016 equal 2016, and there is at least >, =, < to test for each boundary check, there should be at least 6 tests. From checking timestamps.json, there at least 9 test chains in the file.
Wondering if there is full coverage for the first equality check, given you have > and = to test each for the variations of minus 7200 (and then same, you have >, =, < I think that are valuable to test for).
| For the purpose of testing a Timewarp fix, a Timewarp attack was included early on in the history of | ||
| testnet3. An implementer of BIP54 may want to ensure that syncing testnet3 by enforcing BIP54 since | ||
| genesis will treat block `00000000118da1e2165a19307b86f87eba814845e8a0f99734dce279ca3fb029` as | ||
| invalid. |
There was a problem hiding this comment.
Very open question: how hard it would be to coordinate a Murch-Zawy attack variant on testnet3.
4 participants
This introduces test vectors for BIP54. There is one set of vectors per each of the 4 mitigations.
The vectors were generated using the BIP54 implementation against Bitcoin Inquisition available here, as well as a custom miner as a Bitcoin Core unit test available here. Documentation is provided with more details about each set of test vectors and describing how to use them.